Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add basic example with sample files #58

Merged
merged 4 commits into from
Feb 28, 2025
Merged

Conversation

sukritsingh
Copy link
Contributor

Address #49 : Providing a basic example script for running the plumed-openmm plugin in a basic metadynamics simulation.

The script examples/basic-example/plumed-metad-sim.py is a single script contains reads in a system, state, and topology and runs a metadynamics simulation using the PLUMED scripting language to define collective variables. A sample output file from PLUMED, colvar, is also included.

The subdirectory basic-example was created to prevent future confusion if other more advanced examples get added.

@sukritsingh
Copy link
Contributor Author

It looks like a couple of the checks haven't run yet for some reason? @raimis or @peastman do you have any feedback on the examples so this can be merged in?

@peastman
Copy link
Member

I'm not sure what's going on. I just told it to rerun all checks, but I'm not convinced it's really doing anything.

@peastman peastman requested a review from raimis August 1, 2022 16:14
@sukritsingh
Copy link
Contributor Author

Looks it it never really ran the checks either again?

@sukritsingh
Copy link
Contributor Author

Any updates/luck with running these checks?

@sukritsingh
Copy link
Contributor Author

Updated this PR into the latest commit in master!

@peastman let me know if you have any other feedback on this PR! The checks appear to be at the "Expected" stage for some reason.

@roshan2004
Copy link

Sorry, but I still could not find such an example script.

@sukritsingh
Copy link
Contributor Author

I imagine this PR needs to be merged for the example scripts to be added into the main branch. If you're looking for the example scripts I wrote in this PR you could probably find them fastest in the "files changed" tab?

Separately, it does look like all the checks still never ran again, and still need to be rerun prior to merging?

@peastman
Copy link
Member

Sorry, it's my fault for having dropped this. I'm not sure what went wrong with the checks, and since then CI got disabled due to lack of activity. Anyway, I've reenabled it. Let's see if we can get this merged.

A couple of minor suggestions. I don't think this line is ever used for anything:

molecule = Modeller(pdb.topology,pdb.positions)

I also suggest removing this:

# define the platform as OpenCL by default (since CUDA may not always exist)
platform = openmm.Platform.getPlatformByName('OpenCL')

Just let the simulation pick the fastest platform automatically. Otherwise people will copy your code and use OpenCL when they really should use something else!

Sorry again for forgetting this.

@sukritsingh
Copy link
Contributor Author

No worries at all! Everything is Chaos now anyways.

I've made the following changes and pushed them in the latest commit:

  1. Merged in any upstream changes that were made after this PR fork
  2. Deleted the unused molecule object
  3. Removed hardcoding of the platform to OpenCL

It should be good to rerun the workflow checks again now, if you get a chance!

@peastman peastman merged commit 5e59ee1 into openmm:master Feb 28, 2025
0 of 2 checks passed
@peastman
Copy link
Member

Thanks!

CI is failing, but it's not related to this. I'll fix it in a separate PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants